Skip to content

Conversation

xalopp
Copy link
Contributor

@xalopp xalopp commented Nov 22, 2017

No description provided.

@xalopp xalopp requested review from mmoll and aoldemeier November 22, 2017 10:00
README.md Outdated
* in arrays, the key and `=>` operator must be on the same line
* each consecutive variable assignment must align at the assignment operator
* use statements must be sorted lexicographically
* use statements must be sorted lexicographically, the order function can be configured
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how? ;)

@xalopp xalopp force-pushed the use_statement_dictionary_order branch from 4465223 to ca937b5 Compare November 22, 2017 12:12
@xalopp
Copy link
Contributor Author

xalopp commented Nov 22, 2017

@Ma27 👓 ! 😸

@mmoll mmoll requested a review from Ma27 November 22, 2017 12:51
} else if ('string-case-insensitive' === $this->order) {
return strcasecmp($a, $b);
} else {
return $this->dictionaryCompare($a, $b);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method falls back to dictionary as validation approach by default. I've seen that PHPCS loves to mutate public properties using an XML, however I wonder if some kind of validation (e.g. throw an exception in here if order has an invalid value) makes sense to avoid unexpected behavior

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the check to register()

@xalopp xalopp requested a review from Ma27 November 22, 2017 12:52
@xalopp xalopp force-pushed the use_statement_dictionary_order branch from ca937b5 to 0e7bc21 Compare November 22, 2017 13:39
*
* @return int
*/
private function dictionaryCompare($a, $b)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aoldemeier could you check the correctness of this function?

Copy link

@aoldemeier aoldemeier Nov 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After having realized I was confused about the signature of substr I am now convinced that the function is correct (making common sense assumptions about how dictionaryCompare should work)...

@mmoll
Copy link
Contributor

mmoll commented Nov 22, 2017

@xalopp please rebase :)

@xalopp xalopp force-pushed the use_statement_dictionary_order branch from 920f2f9 to daafdf0 Compare November 22, 2017 14:30
class AlphabeticalUseStatementsSniff extends UseDeclarationSniff
{

const NAMESPACE_SEPRATOR_STRING = '\\';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: "SEPRATOR" ->"SEPARATOR"

Copy link

@aoldemeier aoldemeier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me (though fixing the typo would be nice to have)

*
* @return int
*/
private function dictionaryCompare($a, $b)
Copy link

@aoldemeier aoldemeier Nov 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After having realized I was confused about the signature of substr I am now convinced that the function is correct (making common sense assumptions about how dictionaryCompare should work)...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants